fix(plugin-audit): declare preserveAudit on the read-audit ledger write so record-view rows keep the VIEW instant - #16996
Conversation
…te so the row keeps the VIEW instant
`buildRow` writes `created_at: event.viewedAt` on purpose — batching moves the
INSERT off the request path, so `created_at`'s `NOW()` default would stamp a
whole batch with one buffer-drain time. `persistReadAuditRows` wrote that row
under `{ context: { isSystem: true } }` and the module's comment cited that flag
as the mechanism carrying the view instant through.
It never was. `isSystem` exempts a write from the readonly strip; the layer that
decides `created_at` on an insert is `sys_stamp_audit_insert`, which reads
`session.preserveAudit` and has never read `isSystem`. What was actually
carrying the value was the hook's pre-#15964 line, `record.created_at =
record.created_at ?? now` — client-preferred on every insert, no flag required.
#15964 closed that accident, so the ordinary branch now stamps `now`: the flush
instant, on every row in the batch.
Both context keys are kept, for different layers: `isSystem` still carries the
readonly-strip exemption the row needs, `preserveAudit` is what the stamp hook
reads. `preserveAudit` is the ruled historical-import channel (#3493,
reaffirmed by #15964's ruling of 2026-09-06) and a view row's original timeline
is the moment of the view, so this use is inside its declared purpose.
The comment at the `created_at` field is repaired in the same change. It cited
`engine-audit-anchor-write.test.ts`'s "a system-context write is still exempt"
as authority — but that case calls `engine.update`, and `sys_stamp_audit_update`
never writes `created_at` in any branch (`if (isInsert)` guards it), so it is
green whatever the insert path does. The insert path had no pin at all, and the
citation is why nobody re-checked it.
New pin: `read-audit-view-instant-preservation.integration.test.ts` — a real
`ObjectKernel` + `ObjectQLPlugin` + `SqliteWasmDriver`, so the real
`sys_stamp_audit_insert` hook runs. Measured RED before the fix (stored
1788912779588, expected 1551675967891) and green after, with two anti-vacuity
controls that pass on both sides. `read-audit.test.ts`'s docblocks are corrected
too: its engine is a bare `new ObjectQL()` that registers no stamp hook, so its
VIEW-instant case could never have pinned the engine half it claimed.
Fixes #16829
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
…ad-audit-preserve-view-instant
…t repair Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
…new integration test `check:test-source-alias` and `check:type-source-resolution` both went red on the new @objectstack/driver-sqlite-wasm devDependency: without an alias the integration test would resolve the driver through `exports` (dist/), making a test whose whole point is a real driver into a verdict about build state. Both gates prescribe the same self-serve remedy and refuse the alternative (widening their shrink-only registries). The tsconfig `paths` precondition holds: rootDir here is `../..` (packages/) and the redirected source sits at packages/drivers/driver-sqlite-wasm/src, so no TS6059 is owed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
…ource lands in the ESM one The driver paths rule put driver-sqlite-wasm's source into whichever program resolves the specifier. In the BUILD program that source compiles under the root config's CommonJS-bound module setting, where knex-wasm-dialect.ts and wasm-connection.ts are 4 x TS1470 — a verdict about the driver's module semantics billed to this package. Tests now leave that program, exactly as plugin-approvals spells it, and tsconfig.test.json (module esnext / moduleResolution bundler — what vitest really runs) keeps them. Coverage is unchanged: check:type-check-coverage reads the pair, and the typecheck script names both. Measured after: typecheck 0 errors, check:test-typecheck 0 files / 0 errors, check:type-check-coverage / check:type-source-resolution / check:test-source-alias all green, suite 23 files / 326 tests passed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
…ad-audit-preserve-view-instant
📓 Docs Drift CheckThis PR changes 1 package(s): 7 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:
⛔ 3 release-owned page(s) also name something this change touched. These are read-only:
What this run could not see
Coarse fallback — 8 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin c3543fd8a1831c595ce746202a5a41514428afce && git checkout c3543fd8a1831c595ce746202a5a41514428afce
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin fe2b7554a527c27f92582f8259d46c52ac87d4a1 c7f4813468343015e6a617c139bf983e1b57e04d && git checkout -B drift-repro fe2b7554a527c27f92582f8259d46c52ac87d4a1 && git merge --no-ff c7f4813468343015e6a617c139bf983e1b57e04d
node scripts/docs-audit/affected-docs.mjs --json fe2b7554a527c27f92582f8259d46c52ac87d4a1
|
Stand-down record for the two remaining reds — neither is this PR's, and one of them is now a repo-wide condition
✅
|
Fixes #16829
Clause-②: no
sys_audit_log'srecord_viewsrows answer "when did this user look at this record?". Read auditing batches its INSERTs off the request path by design, sobuildRowwritescreated_at: event.viewedAtrather than letting the column'sNOW()default stamp a whole batch with one drain timestamp — up toflushIntervalMsafter the fact, with read order inside the window destroyed.persistReadAuditRowswrote that row under{ context: { isSystem: true } }, and the module's comment cited that flag as what carried the view instant through. It never was.Re-measured on this tree, not inherited
Triage confirmed the anchors on
origin/main e08892da; re-measured here atb834b48e7(branch point). Four of five sit where triage left them; one moved by a line.plugin-audit/src/read-audit.tscreated_at: event.viewedAt,:512:512plugin-audit/src/read-audit.tsthe ledger insert:460:460—{ context: { isSystem: true } }, onlyisSystemobjectql/src/plugin.tsrecord.created_at = preserveAudit ? (record.created_at ?? now) : now;:1123:1124preserveAudit, neverisSystemobjectql/src/engine-audit-anchor-write.test.tsa system-context write is still exempt:216:216— and it callsengine.update.changeset/audit-binder-created-at-unconditional-on-create.mdobjectql/CHANGELOG.mdat17.3.0carries no entry for itConfirmed independently:
sys_stamp_audit_updatepassesisInsert: falseintoapplyToRecord, whosecreated_atassignment is insideif (isInsert). That case is green whatever the insert path does, so it never covered the reliance it was cited for.The change
Both keys, for two different layers.
isSystemstill carries the readonly-strip exemption the row needs;preserveAuditis the one the stamp hook reads.preserveAuditis the ruled historical-import channel (#3493, reaffirmed by #15964's ruling of 2026-09-06) — 「the door audit left open for reinstating an original timeline, not a bypass of it」 — and a view row's original timeline is the moment of the view, so this use sits inside its declared purpose.The test is the substantive half, and it was RED first
packages/plugins/plugin-audit/src/read-audit-view-instant-preservation.integration.test.ts— a realObjectKernel+ realObjectQLPlugin(which is what registerssys_stamp_audit_insert) + realSqliteWasmDriver, read back through the driver's own SQL surface. Shape copied from #16312'spackages/runtime/src/notification-migration-audit-preservation.integration.test.ts, not reinvented.RED before the change — the card's case only:
1551675967891is2019-03-04T05:06:07.891Z, the view instant.1788912779588is the drain instant of that run.GREEN after —
Tests 3 passed (3); whole packageTest Files 23 passed (23) · Tests 326 passed (326).The two passing cases are anti-vacuity controls, green on both sides on purpose:
isSystemwrite does NOT keep a suppliedcreated_at— the card's central claim, asserted on the very write path the ledger uses. It is also the proof the real hook is live in this fixture: in a hook-less harness this is the case that goes red first, by name.context.preserveAuditDOES keep it, on this object and this write path — so a green third case means "the writer declared the channel", not "nothing was ever going to restamp this row".Why a new file rather than the case that already claimed this
read-audit.test.tshas had a case namedrecords the VIEW instant, not the flush instantsince #8992. Its engine is a barenew ObjectQL()over a stub driver — noObjectQLPlugin, therefore no audit stamp hook at all. It is green before #15964, green with the defect live, and green after this change: an instrument that cannot fail. That is the same shape that letmigrate-sys-notification-to-event.test.tsread23 passedfor #16312 while the rows were being restamped.That case is kept but demoted to what it can actually witness — that the writer puts
viewedAton the row it hands the engine — and both its docblock and the file header are corrected. The header previously claimed a third pin this file cannot make, in the same words the source comment used, so leaving it would have kept the false premise alive in a second place.The comment repair
The comment at the
created_atfield citedengine-audit-anchor-write.test.ts's "a system-context write is still exempt" as authority. Both halves were wrong:isSystemexempts a write from the readonly strip and has never been read by the stamp hook, and the cited case is anengine.update. It is replaced with the real mechanism, a pointer to the write that carries it, and a note that what was truly carrying the value was the hook's pre-#15964 linerecord.created_at = record.created_at ?? now.Three commits of harness plumbing, each forced by a measured gate failure
@objectstack/driver-sqlite-wasmas a devDependency ofplugin-audit. No cycle: the driver depends oncore/driver-sql/spec, none of them on this package.check:test-source-aliasred —NEW unaliased artifact import(s). Anchored alias added tovitest.config.ts. Widening the shrink-only registry is explicitly not the remedy.check:type-source-resolutionred —NEW dist-resolved type import(s).pathsrule added totsconfig.json. Its precondition holds:rootDirhere is../..(packages/) and the source sits atpackages/drivers/driver-sqlite-wasm/src, so no TS6059 is owed.typecheckthen red with 4 × TS1470 — thepathsrule puts the driver's source into whichever program resolves the specifier, and in the BUILD program it compiles under the root config's CommonJS-bound module setting, whereknex-wasm-dialect.tsandwasm-connection.tsuseimport.meta. Tests now leave that program, exactly asplugin-approvalsspells it, andtsconfig.test.json(module: esnext/moduleResolution: bundler— what vitest really runs) keeps them. Coverage is unchanged and re-measured:check:type-check-coveragegreen,check:test-typecheck0 files / 0 errors.Ablation — the case can fail, and the tree came back
Run from the committed fix, under a
trap ... EXIT INT TERM, restoring withgit checkout HEAD -- ABSPATH(never the bare form, which restores from the index).Only the card's case moved; both controls stayed green under the mutation, so the ablation is targeted rather than a blanket break. No dist preflight is owed here: the mutated subject is reached by a relative in-package import (
./read-audit.js), so vitest resolves it fromsrc— which the RED itself demonstrates.Gates
Derived from a fresh tree after merging
origin/main(fe2b7554a), reconciled mechanically:Three gates first came back exit 3
PREREQUISITE NOT MET, which is NOT a pass and was not reported as one:check:dual-build-cjs-loads,check:i18nandcheck:type-check-debtall read built output. A full workspace build (turbo run build --filter='./packages/*' --filter='./packages/*/*', 72/72 successful) was run and all three re-run to real verdicts — green.Beyond the derived union:
pnpm lint(whole repo, exit 0 — no narrowing claimed and none needed),pnpm --filter @objectstack/plugin-audit typecheck,pnpm --filter @objectstack/plugin-audit test, and the three artifact-roster families whose roster sits in a directory this diff is in (check:authz-resolver,check:error-code-casing,check:filter-alias-parity) — all green, since their silence is not evidence in either direction.pnpm check:route-envelopewas run explicitly (green), per its known absence from the derived union (#16828). It does not in fact apply: every file in this diff contains zeroc.json(/res.json(/ctx.json(call sites, measured per file.check:partof-closing-keywordwill be RED on this PR, and no author action clears it. The first commit on this branch carries a card-relation trailer in its message, which the contract puts in the PR body only. It was pushed before the mistake was noticed, and this repository forbids amend / rebase / force-push — the gate's own text says so and says the red is then to be READ rather than acted on. Recorded here rather than left for a reviewer to discover. The relation itself is correct and unambiguous: the trailer names this same card and nothing else, and the body above declares it once.验收备注
Triage's acceptance criteria, carried verbatim:
sys_stamp_audit_insert钩子 —— ⛔ 不是任何跳过钩子的替身。 → met: realObjectKernel+ObjectQLPlugin+SqliteWasmDriver, with an explicit anti-vacuity control that goes red if the hook is not live.expected 1788912779588 to be 1551675967891), then the change; both readings above, plus an ablation reproducing the red from the committed state.packages/runtime/src/notification-migration-audit-preservation.integration.test.ts。⛔ 不必重新发明。 → met: copied, including its three-reading structure and its two anti-vacuity controls.And the two constraints the dispatch carried as binding:
Third-site sweep — the extra deliverable
Re-ran (not inherited) the eight-row
created_atenumeration whose incompleteness produced this card. Result: no third RELIANT site; exactly two exist and both are known. Posted to #16312 with the method, the positive control, and the four sites that table omitted: #16312 (comment)persistReadAuditRowsandbuildRoware different functions ~50 lines apart. Reliance is a dataflow fact, not a proximity fact.Out of scope — noted, not filed
packages/services/service-storage/src/metadata-store.ts:321builds{ created_at: now, updated_at: now, ...rec }with the spread LAST, so a caller-suppliedcreated_atwould win and then be silently restamped by the hook. Not a defect today — bothcreateFilecallers supply none — and not a reproducible one, so ⛔ not filed. A latent channel of the same class, recorded here for whoever adds the third caller.??: it is a pin that claims an ENGINE guarantee from a harness that boots no engine. That is a repo-wide question about test-harness posture, not a defect, so ⛔ not filed as one.🤖 Generated with Claude Code
https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
Generated by Claude Code